Allow running in node environment #119
Conversation
src/events.js
Outdated
| } | ||
| const window = node.ownerDocument.defaultView | ||
| if (typeof window[EventType] === 'undefined') { | ||
| window[EventType] = class extends window.Event {} |
There was a problem hiding this comment.
Here's relevant line, btw. Previously this events were shimmed only in document.js helper
| // Clipboard Events | ||
| copy: { | ||
| EventType: ClipboardEvent, | ||
| EventType: 'ClipboardEvent', |
There was a problem hiding this comment.
Probably because of changes like this bundler didn't bother to shim things like ClipboardEvent (not that it should as such shimming won't work on custom jsdom instances, just on global window)
There was a problem hiding this comment.
The bundler doesn't shim anything.
There was a problem hiding this comment.
I tested this locally with react-testing-library's tests and they passed. If this passes CI then I'll merge it.
Thanks for your work on this @sheerun 👍
|
I'm not sure why the test is breaking, but I do want to use |
| return element.dispatchEvent(event) | ||
| } | ||
|
|
||
| Object.entries(eventMap).forEach(([key, {EventType = Event, defaultInit}]) => { |
There was a problem hiding this comment.
They were just defaulting to Event before due to EventType = Event in the function params above, when EventType was undefined (ie, InputEvent didn't exist before jsdom 12 so was undefined). Now that they are all changed to strings, like 'InputEvent', it's always defined so never defaulted over to 'Event'.
Your new fix of
if (typeof window[EventType] === 'undefined') {
window[EventType] = class extends window.Event {}
}
const event = new window[EventType](eventName, eventInit);
works, but we could also just do something like
const eventType = window[EventType] || window.Event;
const event = new eventType(eventName, eventInit);
if we wanted to keep the same behavior of not shimming.
There was a problem hiding this comment.
Yeah, my changes address this.
There was a problem hiding this comment.
oh, cool. Bit behind on the code updates then ^_^
I definitely like name EventConstructor over a weird lowercased eventType.
|
The error was because document has no ownerDocument and it can be passed as a container. I've fixed the code to account for this case. |
|
We've got some merge conflicts. Could you look at those please? |
src/wait-for-element.js
Outdated
| timer = setTimeout(onTimeout, timeout) | ||
| observer = new MutationObserver(onMutation) | ||
| const window = container.ownerDocument.defaultView | ||
| const MutationObserverConstructor = window.MutationObserver || MutationObserver |
There was a problem hiding this comment.
This code is not necessary as @sheerun/mutationobserver-shim already uses global MutationObserver if available
There was a problem hiding this comment.
Oh nice! Yeah, when you work on the merge conflicts, could you fix that?
Thanks for all the work your putting into this 👍
|
|
||
| function getDocument() { | ||
| if (typeof document === 'undefined') { | ||
| throw new Error('Could not find default container') |
There was a problem hiding this comment.
This code defeats purpose of this PR as this while this PR adds ability for this library to work without global document, this code throws an error when so..
There was a problem hiding this comment.
No, this is a more elegant way of what you were doing before. To avoid running this code all you have to do is provide a container to waitForElement. If you don't, it'll throw the error (which is what was happening before).
There was a problem hiding this comment.
Hmm ok it seems fine, I didn't realize JS evaluates default values lazily
|
I'll send another one shortly |
Fixed version of previous PR, I now automatically shim not existing event types on window